[discussion] Saved state is not meant to be user editable. It is meant to save the state of a running script so that it can pick up where it left off next time it runs. This sounds more like configuration and should use the configuration feature.

I interpreted things differently; I see the global config files (experimental_config & europi_config) as module-wide settings, while the saved_state files are effectively config files with a per-script focus. Adding a script-specific setting to e.g. experimental_config feels like overkill, since that setting would never be used anywhere else.

Creating separate saved-state & config files for each script also feels counter-intuitive to me. The saved state is the current configuration of the script. This particular feature is just a sneaky way of adding an extra bit of functionality that otherwise wouldn't be supported.

If that's how you are interpreting these features then the documentation for them needs to be improved. The firmware level config files are certainly module wide settings, but individual scripts can also have configuration, see the turing-machine and diagnostic script for examples.

This particular feature is just a sneaky way of adding an extra bit of functionality that otherwise wouldn't be supported.

My assumptions about why this feature wouldn't otherwise be supported is that the limited number of hardware UI elements on the EuroPi make adding these features difficult with out making it very menu driven. This is explicitly one of the things that the configuration system was meant to address.

Please see the following issue and PRs for details of the motivations for these features. I will open a PR that pulls these details into the documentation.

#163
#220
#185

Fair enough. I'll move that one item out of the saved state & create a separate config file for it.

EDIT
Done

[required] This documentation is meant to be understood by non-developer users of the script. As such, I don't think that describing a button as either high or low voltage is clear. I think it would be more clear to describe it in terms of the button being pressed or not.

I'll reword this to make it less developery-sounding.

Done.

[question] These timestamps don't seem to be read anywhere. Are the meant to be a part of the class's API? If so they should be documented.

Yes, they're part of the API. They aren't used here but they are used in other multi-threaded scripts I have on the go. I'll add some additional documentation.

Added an additional section in the class header to explain the additional fields.

[discussion] I haven't played with this script yet, but I think that this would be useful/fun: Have you considered having the spread be zero in the center, adding the ability to both slow down and speed up the related clocks?

An interesting thought. Probably wouldn't be too hard to change the knob behaviour to allow the spread to speed up or slow down cv2-6.

I played around with this a little bit. In the end, I don't think I actually like this change; with the current implementation it's very easy to "lock" the waves' relative phases by just turning the spread control all the way to the left. That feels like a useful feature that gets lost if you need to centre the knob to set the spread to zero; the slightest imprecision will cause the waves to start spreading over time.

I may try it out some more later, but for now I think I'm leaning towards keeping the behaviour as it is right now.

[optional] You could use these constants as the values in the above dictionary to reduce the number of places where the code has to match up a shape with its value.

Done

[required] The configuration file and the saved state file are two different things in the europi firmware. It's confusing to have the documentation here call it a configuration file and then read from the saved state file. Ditto for the save() method below.

Like I said above, the saved_state file (in my interpretation) is the same thing as a per-script configuration file.

Fixed